fix(rivetkit-core): cancel driver alarm before sqlite teardown on destroy - #5530

Stack for rivet-dev/actors

• #5557 docs(container-runner): sync deploy docs with runner source
• #5556 fix(container-runner): read cgroup v1 memory usage under gvi …
• #5548 feat(container-runner): log version and git sha at actor sta …
• #5549 feat(container-runner): setup instance memory and cpu usage …
• #5546 fix(rivetkit): keep actor event loop alive through shutdown …
• #5539 feat(container-runner): report actors as crashed on unexpect …
• #5536 feat(container-runner): log unexpected platform SIGTERM as a …
• #5530 fix(rivetkit-core): cancel driver alarm before sqlite teardo … 👈
• #5531 feat(container-runner): keep instance warm instead of self-e …
• #5533 chore(container-runner): default stop grace to 10s
• #5528 fix(rivetkit): log and surface actor startup failures
• #5558 fix(sdks): drop rivet-util metrics dep from protocol crates …
• main

Get stack: forklift get 5530
Push local edits: forklift submit
Merge when ready: forklift merge 5530

_{change qprwzozl}

Review

Re-verified after rebase (commit c0744a7, same diff as prior review).

Small, well-scoped fix. The reordering is correct and fixes a real race.

What the change does

Moves ctx.cancel_driver_alarm_logged() (Destroy-only) to run right after sync_alarm_logged() and before wait_for_pending_alarm_writes() / cleanup_sqlite(), instead of after SQLite teardown. cancel_driver_alarm_logged calls set_alarm_tracked(None), which spawns a task that persists last_pushed_alarm to SQLite once the envoy ack arrives (persist_last_pushed_alarm in actor/state.rs:593, backed by internal_storage::persist_last_pushed_alarm). With the old ordering, that write ran after cleanup_for_shutdown had already closed the local SQLite backend (sqlite/mod.rs::close_backend), so it could fail against a closed connection (surfacing as sqlite.transaction_closed) and was never awaited by shutdown anyway. Moving the call earlier means wait_for_pending_alarm_writes() now drains and awaits that specific pending write before SQLite is torn down, closing the race.

This also brings Destroy in line with the existing pattern already used for sync_alarm_logged()'s pending write (already awaited via wait_for_pending_alarm_writes() before cleanup), so it's consistent with precedent rather than a new architecture. The match reason { Destroy => ..., Sleep => {} } arms are exhaustive per repo convention (no _ fallthrough), and the untouched Sleep path (cancel_local_alarm_timeouts() after cleanup, keeping the persisted engine alarm armed for the next instance) still looks correct.

Minor notes (non-blocking)

• Test coverage: destroy_shutdown_still_clears_driver_alarm_after_cleanup and sleep_shutdown_preserves_driver_alarm_after_cleanup in tests/task.rs only assert test_driver_alarm_cancel_count(), which is unchanged by this reordering. Nothing in the diff would fail if the old (buggy) ordering were reintroduced. Consider a regression test that specifically exercises "the pending alarm-cancel write completes before SQLite cleanup starts" (or fault-injects to reproduce the old sqlite.transaction_closed), so the fix can't silently regress. Note the destroy test's name (..._after_cleanup) is now slightly stale since the cancel happens before cleanup, worth a rename in the same pass if a new test is added.
• New blocking dependency on shutdown path: awaiting the pending alarm write means Destroy shutdown now blocks on the envoy round-trip ack for set_alarm(None) before SQLite cleanup proceeds. This mirrors existing behavior for the sync_alarm_logged write, so it isn't a new risk class, but worth confirming there's an overall shutdown deadline/timeout upstream so a slow or unresponsive envoy can't stall actor destroy indefinitely.
• Comment style: the added comment includes a parenthetical ((rather than after cleanup_sqlite)) that the repo's CLAUDE.md style guidance discourages ("avoid fragmented structures with parentheticals and dashes"). Minor, but could be rephrased as its own sentence. Also, it explains both the Destroy-path reordering here and the Sleep-path behavior further down (cancel_local_alarm_timeouts after cleanup); consider moving the Sleep explanation to a short comment at that call site so each match arm's reasoning stays local.

No security or naming/style concerns beyond the above. The fix is narrowly targeted and doesn't touch unrelated code paths.